test(northwind): poll for async auth-cache propagation after alter_role#1475
test(northwind): poll for async auth-cache propagation after alter_role#1475kriszyp wants to merge 3 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces polling using a waitFor helper in several integration tests to account for asynchronous permission propagation to the worker auth-cache. The feedback suggests ensuring that the returned response from waitFor is explicitly asserted for the expected HTTP status code (e.g., 200 or 403) before inspecting the response body. This prevents silent test failures or unhelpful TypeErrors if the polling times out.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Reviewed; no blockers found. |
Three stale-auth-cache races caused ~50% CI failure rate with varying line numbers. alter_role returns HTTP 200 after the DB write but before the async ITC permission-cache invalidation reaches all worker threads, so the next request can hit a worker with stale permissions. Affected spots: - CSV upsert with newly granted attribute permissions - Two cross-schema SQL JOINs that require the updated role to be visible Each affected request is now wrapped in the existing waitFor() poll helper, retrying until the permission change is visibly effective. No product code changed. Test-only stabilization. Verified locally 5/5 runs (560 pass). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…fails clearly Addresses Gemini review: waitFor returns the last value on timeout; assert the expected status (200/403) before body assertions so a propagation timeout surfaces a clear failure instead of a confusing TypeError or silent pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The previous poll predicate (status === 403) was satisfied at an intermediate/stale role-propagation state — a 403 with empty unauthorized_access and only one invalid_schema_item — so the test still flaked after the waitFor was added (CI on the fix commit still failed: "0 !== 1" on unauthorized_access.length). alter_role propagates to worker auth-caches via async ITC, and a 403 alone does not prove the *current* restriction is fully visible. Poll until the full expected shape is present (1 unauthorized table = dog AND all 3 invalid_schema_items) before asserting. Verified locally 3/3. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
kriszyp
force-pushed
the
kris/flake-northwind
branch
from
86aa495 to
c0b256b
Compare
|
Rebased onto current The Now polls until the full expected shape is present (1 unauthorized table = Note on remaining red shards: those failures are in other test files sharing the shard (e.g. — Claude Opus 4.8 |
|
Follow-up issue for the durable product-side fix: #1497 (cross-worker ITC propagation barrier). — Claude Opus 4.8 |
1 participant
De-flakes
northwind.test.mjs, which was failing ~50% of CI runs with varying line numbers due to three stale-auth-cache races.Root cause:
alter_rolereturns HTTP 200 after the DB write but before async ITC permission-cache invalidation reaches all worker threads. The test assumed the role change would be immediately visible on the next request, but a worker with a stale cache could serve it instead.Fixes (test-only, no product code changed): Three affected requests are now wrapped in the existing
waitFor()poll helper, retrying until the permission change is visibly effective:Verified: 5/5 local runs, 560 tests pass.
Generated with Claude Opus 4.8